Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add changes to support URL redirection. #50

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mrigaya
Copy link

@mrigaya mrigaya commented Mar 9, 2018

Server URL redirection is not handled in nopoll.

As per RFC https://tools.ietf.org/html/rfc6455#section-4.2.2, library/server should handle non 101 status code received during handshake. Currently nopoll is not handling this feature, whenever it receives non 101 status code will close the connection and not supporting any other functionality like URL redirection in case of 3xx status code is received.

These changes may handle this scenario, if server sends redirection URL to the user/client using a 3xx status code, the user/client may establish connection with the redirected URL.

Review these changes and Please let me know if it requires any modification/suggestion from your end.

@mrigaya
Copy link
Author

mrigaya commented Mar 20, 2018

@francisbrosnan Any suggestion on this pull request?

@francisbrosnan
Copy link
Member

Hello @mrigaya
I think the patch looks good. It will require additional work to create a regression test to ensure quality and feature tracking across releases.
I've send you a private message because this patch is big enough to be considered as a contribution instead of a fix or similar, so it needs a CLA.
I've sent you all details by mails to continue with the process.
Thanks for your time and effort.
Best Regards.

@schmidtw
Copy link
Contributor

@francisbrosnan What do you think about this PR, can we move forward with it?

@schmidtw
Copy link
Contributor

@mrigaya & @francisbrosnan Has any progress been made on the CLA agreement? We'd like to use this code, but it's stuck as a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants